Update packagist service to use TLS#1049
Conversation
|
@Seldaek you need to update the tests too |
|
Oh I missed the failed build thanks |
|
I suggest adding a test ensuring that packagist.org is forced as https |
|
Done |
|
This still feels somewhat error prone because of the regexes. I with we could specify to Either way, you do have an |
|
@kdaigle yeah I am not sure why that is there tbh.. I removed that line, but I don't think we should necessarily force TLS always because if people use that hook with custom installs they might not have an https endpoint, so I only force it for packagist. And yes it's a bit messy with the regex but I don't see any other way to force all existing configs that might have http://packagist.org hardcoded in them to do the upgrade. |
There was a problem hiding this comment.
Can you use \z to denote the end of the string instead of $?
Can you also use \A to denote the beginning of the string instead of the ^ here?
Thank you!
There was a problem hiding this comment.
I don't mind doing it, but I am curious why? :)
There was a problem hiding this comment.
@Seldaek with the ^ and $, it'd be possible to add a newline into the string and still get a correct match since those match per "line" and not "per string".
There was a problem hiding this comment.
That's only true with the m modifier though? Unless ruby has that on by default, I don't know it much :)
There was a problem hiding this comment.
The /m flag only changes the meaning of . to match newline characters. Otherwise, this is a good way to make sure we don't over match.
|
cc @gregose @mastahyeti @ptoomey3 @oreoshake for a review of this PR before merging |
|
👍 - Thanks for setting 🔥 to |
|
@ptoomey3 I am pretty sure it was added originally because the hook started off as a copy paste of another hook.. Lots of non-ruby devs contributing to this and doing things by following (sometimes bad) examples. |
Update packagist service to use TLS
|
Thanks! We'll get this deployed in the next few days. 🍰 |
|
A push on the "Test Service" may help. |
5 participants
No description provided.